Skip to content

Conversation

@NandanaRaol
Copy link
Contributor

This pull request introduces support for composite primary keys in Django 5.2 and later for the MSSQL backend. It includes changes to conditionally import CompositePrimaryKey, modify DatabaseFeatures, and update schema creation logic to handle composite primary keys.

Support for Composite Primary Keys:

  • mssql/features.py: Added conditional import of CompositePrimaryKey based on Django version and updated DatabaseFeatures to include supports_composite_primary_keys. Also set supports_tuple_lookups to False as a fallback when composite primary keys are supported. [1] [2]

  • mssql/schema.py: Added conditional import of CompositePrimaryKey and updated the create_model method to detect and handle composite primary keys. This includes generating a PRIMARY KEY SQL clause for composite keys and inserting it into the constraints list during table creation. [1] [2] [3]

Copilot AI review requested due to automatic review settings June 17, 2025 04:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for composite primary keys for the MSSQL backend in Django 5.2 and later. It conditionally imports CompositePrimaryKey, updates database features to reflect composite primary key support, and revises schema creation logic to generate a proper PRIMARY KEY clause for composite keys.

  • Introduces conditional imports for CompositePrimaryKey based on the Django version.
  • Updates DatabaseFeatures to include composite primary key support and disable tuple lookups when applicable.
  • Enhances schema creation logic to handle composite primary keys by generating and inserting a PRIMARY KEY SQL clause.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
mssql/schema.py Added logic to generate a composite primary key SQL clause during table creation.
mssql/features.py Conditionally sets support flags for composite primary keys and tuple lookups.

# CompositePrimaryKey support is only available in Django 5.2 and later
supports_composite_primary_keys = django_version >= (5, 2)
if django_version >= (5, 2) and isinstance(CompositePrimaryKey, type):
# Set fallback tupple lookup support
Copy link

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misspelling: 'tupple' should be corrected to 'tuple'.

Suggested change
# Set fallback tupple lookup support
# Set fallback tuple lookup support

Copilot uses AI. Check for mistakes.
@NandanaRaol NandanaRaol changed the title FEAT: Added composite primary key support FEAT: Support 5.2- Added composite primary key support Jun 17, 2025
@NandanaRaol NandanaRaol changed the title FEAT: Support 5.2- Added composite primary key support FEAT: Support 5.2- Added composite primary key Jun 17, 2025
@bewithgaurav
Copy link
Collaborator

For this task there is a dependency on subquery lookup using tuple
The support for tuple is subquery lookup is an open PR/Issue as of now in Django Repo - django/django#19565
Will be parking this until that fix is release from Django side

@iamjameswalters
Copy link

Looks like django/django#19565 was merged.

@bewithgaurav thanks so much for stepping in to help this project along!

@Wedge009
Copy link

Is this something that's missing in mssql-django 1.6?

@Wedge009
Copy link

It turns out this resolves my issue in #471. It's a shame this wasn't included for the long-awaited 1.6 release. Is there anything outstanding here? If not, can it be merged and published as soon as possible, even as a point release or something? It would be a real shame if we have to wait yet another 16 months before the next release...

@sumitmsft
Copy link
Collaborator

Hi @Wedge009 - Apologies for the delay in merging this PR. We are pending some tests on this PR and some other housekeeping. We will do this asap and hopefully you'll be unblocked on your issue. Please allow us sometime to bring this to life.

Thanks for your understanding.

@Wedge009
Copy link

Appreciate the response - I didn't see any indication that composite primary key support was incomplete in the release notes or limitations documentation. As a work-around, I have manually patched my local copy with this PR, but obviously that isn't a feasible long-term solution.

I do hope the next release is within a reasonable time. Thanks.

@sumitmsft
Copy link
Collaborator

@Wedge009 I acknowledge that we missed to write this limitation explicitly in the documentation but there was a slight reference to it in the readme. However, we should have been more explicit to avoid any confusion.

Nevertheless, I'm glad that you were able to get unblocked on your local copy and we will make every effort to release this soon. It won't be a long wait that I can assure you.

@Wedge009
Copy link

Hello, any progress on this? It's not been good having to manually patching mssql-django every time I need to set-up a new virtual environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants